-
Notifications
You must be signed in to change notification settings - Fork 11
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add Docker image for cargo-pgo
#52
Conversation
Hi, thanks a lot for the PR! Regarding the LLVM installation approach, have you tried that BOLT indeed works, both for instrumentation and then for generating samples? I haven't tried with LLVM 18, but with previous versions, the installed |
Yeah the BOLT also works well. You can test it locally using Firstly, build the image and get into the container: git clone https://github.com/bioinformatist/cargo-pgo.git && cd cargo-pgo
docker build -t pgo .
docker run --entrypoint bash -v $(pwd):/workdir --rm -it pgo Then in the container: cargo pgo build
/workdir/target/x86_64-unknown-linux-gnu/release/cargo-pgo pgo info
cargo pgo bolt build --with-pgo
/workdir/target/x86_64-unknown-linux-gnu/release/cargo-pgo-bolt-instrumented pgo info
cargo pgo bolt optimize --with-pgo Below shows the result: I believe the issue lies in the suffix of the binaries (like the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, I didn't notice the symlinks. That's great, thanks! I left a few comments, mostly I want to simplify the PR a bit.
Got that! I'll finish it ASAP. |
No need to hurry ;) |
Then run this in your project directory to create a container: | ||
|
||
```bash | ||
docker run -v $(pwd):/workdir --rm -it cargo-pgo |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The resulting files will be owned by root
by default 🤔 Maybe we should somehow mention setting a different user ID. But it's probably outside of scope for this README.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! :)
Hey Jakub! Sorry for such a late PR!
To deploy an image with your Docker Hub account, you may need to generate an access token and then add your account and token to this repo's secrets.
I've tested the new workflow with a test-use repository and it works well. These repositories will be deleted once this PR is merged 😄